execution/vm: fold EVMInterpreter into EVM #18765
Conversation
yperbasis
requested review from
Giulio2002,
antonis19,
canepat,
Copilot and
taratorio
There was a problem hiding this comment.
Pull request overview
This PR refactors the EVM architecture by folding the EVMInterpreter struct into the EVM struct, eliminating an unnecessary layer of indirection. This is a cherry-pick from go-ethereum that simplifies the codebase by consolidating interpreter functionality directly into the EVM.
Changes:
- Merged EVMInterpreter fields and methods into the EVM struct
- Updated all opcode function signatures to accept
*EVMinstead of*EVMInterpreter - Removed the Interpreter interface and related test infrastructure
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| execution/vm/evm.go | Added interpreter fields (jt, depth, hasher, readOnly, returnData) to EVM struct and removed interpreter indirection |
| execution/vm/interpreter.go | Removed EVMInterpreter struct, Interpreter interface, and VM struct; refactored Run method to be on EVM |
| execution/vm/jump_table.go | Updated executionFunc signature to accept *EVM parameter |
| execution/vm/instructions.go | Updated all opcode implementations to accept *EVM instead of *EVMInterpreter |
| execution/vm/eips.go | Updated EIP-specific opcodes to accept *EVM parameter |
| execution/vm/instructions_test.go | Removed EVMInterpreter references from tests |
| execution/vm/mock_vm.go | Removed test mock file no longer needed |
| execution/vm/evm_test.go | Removed interpreter-specific tests |
| execution/tracing/tracers/logger/logger_test.go | Updated to call evm.Run directly |
| execution/tracing/tracers/js/tracer_test.go | Updated to call evm.Run directly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| v, overflow = uint256.FromBig(evm.Context.Difficulty) | ||
| if overflow { | ||
| return pc, nil, errors.New("interpreter.evm.Context.Difficulty higher than 2^256-1") | ||
| return pc, nil, errors.New("evm.Context.Difficulty higher than 2^256-1") |
There was a problem hiding this comment.
The error message refers to "evm.Context.Difficulty" which is misleading. The error occurs when converting the difficulty value, but the message structure suggests a field path. Consider rewording to "difficulty value exceeds 2^256-1" for clarity.
| return pc, nil, errors.New("evm.Context.Difficulty higher than 2^256-1") | |
| return pc, nil, errors.New("difficulty value exceeds 2^256-1") |
| evm.IntraBlockState().AddBalance(beneficiaryAddr, balance, tracing.BalanceIncreaseSelfdestruct) | ||
| evm.IntraBlockState().Selfdestruct6780(callerAddr) | ||
| if evm.Config().Tracer != nil && evm.Config().Tracer.OnEnter != nil { | ||
| evm.config.Tracer.OnEnter(evm.depth, byte(SELFDESTRUCT), scope.Contract.Address(), beneficiaryAddr, false, []byte{}, 0, balance, nil) |
There was a problem hiding this comment.
Inconsistent field access pattern: uses lowercase evm.config instead of uppercase evm.Config() method used elsewhere in the same function (lines 1298, 1301). This inconsistency could lead to maintainability issues if the config access pattern needs to change.
| if interpreter.evm.Config().Tracer != nil && interpreter.evm.Config().Tracer.OnExit != nil { | ||
| interpreter.cfg.Tracer.OnExit(interpreter.depth, []byte{}, 0, nil, false) | ||
| if evm.Config().Tracer != nil && evm.Config().Tracer.OnExit != nil { | ||
| evm.config.Tracer.OnExit(evm.depth, []byte{}, 0, nil, false) |
There was a problem hiding this comment.
Inconsistent field access pattern: uses lowercase evm.config instead of uppercase evm.Config() method used elsewhere in the same function (lines 1298, 1301). This inconsistency could lead to maintainability issues if the config access pattern needs to change.
| @@ -1,436 +0,0 @@ | |||
| // Copyright 2024 The Erigon Authors | |||
Cherry pick ethereum/go-ethereum#32352